Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix typeassert error in read_refs #170

Closed
wants to merge 3 commits into from
Closed

Fix typeassert error in read_refs #170

wants to merge 3 commits into from

Conversation

DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Jul 18, 2017

This patch fixes a typeassert error that arises in the read_refs function in src/JLD.jl:482. The error arises when, for example, eltype(out) is Array{Array,1} but typeof(read_ref(f, refs[i])) is Array{Array{Float32,1},1}. Array{Float32,1} is a subtype of Array, but Array{Array{Float32,1},1} is not a subtype of Array{Array,1} because in Julia type parameters are invariant.

julia> Array{Float32,1} <: Array
true
julia> Array{Array{Float32,1}, 1} <: Array{Array, 1}
false

Thus, if you try to assign a value of type Array{Array{Float32,1},1} to out[i], but out[i] is expecting a value of type Array{Array,1}, then a TypeAssert is thrown.

The simple workaround is to change the type of out from Array{T} to Array{Any}.

Fixes #154 in this repo as well as pluskid/Mocha.jl#219 and pluskid/Mocha.jl#229. (All are the same issue.)

@DilumAluthge DilumAluthge changed the title Fix typeassert error in read_refs (#154) Fix typeassert error in read_refs Jul 18, 2017
@DilumAluthge
Copy link
Member Author

Hmmmm so now it passes on some platform/version combos and fails on others. Are these fails related to this patch, or due to other causes?

src/JLD.jl Outdated
end
return out
else
throw(e)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rethrow would preserve the backtrace

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed throw(e) to rethrow(e)!

@vtjnash
Copy link
Contributor

vtjnash commented Jul 29, 2017

Thus, if you try to assign a value of type Array{Array{Float32,1},1} to out[i], but out[i]is expecting a value of typeArray{Array,1}, then a TypeAssert is thrown.

An assignment like this should cause it to convert one type to the other, and not throw a Type Error

@DilumAluthge
Copy link
Member Author

I saw your comment here. Does PR #169 fix issue #154?

@DilumAluthge
Copy link
Member Author

I'm closing this PR for now; I'll reopen if the issue reappears!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: read_refs: in typeassert When loading Mocha Snapshot
3 participants